Skip to content

feat!: remove module provider "aws" block — caller owns credentials (v5.0.1) - #57

Merged
obezpalko merged 3 commits into
mainfrom
feat/v5.0.1-remove-module-aws-provider
Jul 31, 2026
Merged

obezpalko merged 3 commits into
mainfrom
feat/v5.0.1-remove-module-aws-provider

Conversation

@obezpalko

@obezpalko obezpalko commented Jul 31, 2026 •

Copy link
Copy Markdown

User description

Why

The module declared its own provider "aws" (no assume_role). Because a child module's provider config wins for its own resources and ignores the caller's, module.comet's resources ran on the module's credential-less provider — ambient atlantis-sa (094792403439) instead of the wrapper's assumed STSaaS role (947208553405) → the 403s that atlantis.yaml works around by assuming the role in a shell step and exporting session creds. It also means a local AWS_PROFILE on the wrapper is ignored by the module. This is the deprecated pattern.

Change

Delete the module's provider "aws" block. Keep the provider requirement (required_providers). Module resources now inherit the caller's provider → the wrapper fully controls credentials/region/tags.

Verified: no configuration_aliases / provider = aws.* / aliased providers anywhere in the module → single-provider inheritance is safe. region/common_tags/environment_tag remain used elsewhere (27/63/1 refs). init+validate pass.

Wrapper migration (at ?ref bump to v5.0.1)

  1. Wrapper provider "aws" must carry region + assume_role/profile — stsaasuat already does (var.provider_assume_role_arn).
  2. Move default_tags (Terraform / Environment / common_tags) onto the wrapper provider — the module no longer sets them.
  3. The atlantis.yaml assume-role-and-export-creds shell hack can be replaced with a normal provider-level assume_role.

Folds into the v5.0.x line → tag v5.0.1 after merge, and the in-flight stsaasuat cutover bumps to v5.0.1 in one pass.

🤖 Generated with Claude Code


Generated description

Below is a concise technical summary of the changes proposed in this PR:
Remove the module-level provider "aws" configuration so comet_eks resources inherit the caller’s credentials, region, and default tags from the wrapper provider. Update the AWS provider requirements and module docs to match that ownership model, and add a plan-time check for var.region-scoped IAM behavior.

TopicDetails
Provider inherit Remove the child-module AWS provider config so the wrapper’s provider "aws" owns credentials, region, and tags for comet_eks resources.
Modified files (4)
  • modules/comet_eks/README.md
  • modules/comet_eks/versions.tf
  • providers.tf
  • versions.tf
Latest Contributors(2)
UserCommitDate
alexb@comet.comfix(eks): guard provid...July 31, 2026
jms200upgrade upstream eks/a...June 12, 2026
Region guard Add a region consistency guard that fails if the inherited AWS provider region does not match var.region for Karpenter and EC2-scoped grants.
Modified files (1)
  • modules/comet_eks/main.tf
Latest Contributors(2)
UserCommitDate
alexb@comet.comfix(eks): guard provid...July 31, 2026
CRThazeMerge pull request #52...July 29, 2026
Review this PR on Baz | Customize your next review

…v5.0.1)

BREAKING (operational, not API): the module no longer declares its own
provider "aws". As a child module, a self-declared provider block takes
precedence for the modules resources and IGNORES the callers provider,
stripping the wrapper control over credentials (assume_role / profile /
region) and default_tags — the deprecated pattern, and the root cause of the
STSaaS-account 403 workaround in atlantis.yaml (the module credential-less
provider ran as ambient atlantis-sa instead of the assumed STSaaS role).

The module keeps its provider *requirement* (versions.tf required_providers).
Now every module.comet resource inherits the CALLERs provider.

Wrapper migration required when bumping to v5.0.1:
  * ensure the wrapper provider "aws" carries region + assume_role/profile
    (stsaasuat already does, via var.provider_assume_role_arn)
  * move default_tags (Terraform / Environment / common_tags) onto the wrapper
    provider — the module no longer sets them
  * the atlantis.yaml assume-role-and-export-creds shell hack for the stsaas
    workflow can then be replaced with a normal provider-level assume_role

No aliased/region-specific providers exist in the module (checked: no
configuration_aliases, no provider = aws.*), so single-provider inheritance is
safe. terraform init + validate pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread providers.tf
Comment thread providers.tf
…am eks)

Replace the loose/inconsistent aws constraints (~> 6.0 root, >= 6.0 comet_eks)
with an honest floor: >= 6.52, the minimum required by the pinned
terraform-aws-modules/eks ~> 21.24. Anything looser was misleading — init could
never resolve below 6.52 anyway.

No upper bound and no bad-build exclusion (e.g. != 6.57.0) in the module: those
are deployment policy and stay in the wrappers. Effective resolved constraint
with the stsaasuat wrapper is >= 6.52, < 7.0, != 6.57.0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread modules/comet_eks/versions.tf
…s README

Addresses baz review on #57:

* Region consistency (medium): with the module provider block removed, the
  provider region now comes from the caller while several resources still key off
  var.region as a string — notably the Karpenter controller IAM policy (EC2 ARNs
  scoped to arn:aws:ec2:${var.region}:... and an aws:RequestedRegion == var.region
  condition). If the caller provider region diverges, those grants target the wrong
  region and deny. Add data.aws_region.current + a terraform_data precondition that
  fails fast at plan when they mismatch. (Kept var.region in the ARNs — it is the
  intended region; the guard just enforces the provider agrees. Lighter and clearer
  than threading data.aws_region through every ARN.)

* Stale docs (low): modules/comet_eks/README.md advertised aws >= 5.0 and a
  kubernetes >= 2.10 requirement. Update to aws >= 6.52 (matches versions.tf) and
  drop kubernetes (removed in v5.0.0); add the time provider the module actually uses.

Skipped baz driver-README direct-consumer note: this module is only consumed as a
wrapper child (no backend of its own); the new precondition already fails fast on a
region mismatch, covering the substantive concern.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@obezpalko
obezpalko merged commit ec36af0 into main Jul 31, 2026
5 checks passed
Comment thread modules/comet_eks/main.tf
Comment on lines +1085 to +1087
condition = data.aws_region.current.region == var.region
error_message = "The AWS provider region (${data.aws_region.current.region}) must match var.region (${var.region}). This module inherits the caller's provider — set the wrapper's provider \"aws\" { region = ... } to the same region you pass as region/eks region, or the Karpenter IAM ARNs and aws:RequestedRegion condition will target the wrong region."
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

terraform_data.region_consistency references data.aws_region.current.region in both the precondition and error_message, but that attribute isn't exported by hashicorp/aws v6 — the region name lives in data.aws_region.current.id (or .name depending on version) — so Terraform errors with Unsupported attribute at plan time before the fail-fast check can even run. Should we switch both references to the correct exported attribute (e.g. data.aws_region.current.id) and update the error_message accordingly?

Severity web_search

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents
Before applying, verify this suggestion against the current code and the installed AWS
provider version. In modules/comet_eks/main.tf around lines 1082-1089, inside the
`terraform_data.region_consistency` `lifecycle { precondition { ... } }` block, the
`condition` and `error_message` reference `data.aws_region.current.region`, which is not
a valid exported attribute on `data "aws_region" "current"` for `hashicorp/aws` v6 (the
region name is exposed as `.id`, or `.name` in some provider versions). Update both the
`condition` comparison against `var.region` and the `error_message` interpolation to use
the correct attribute (`data.aws_region.current.id`), so the region-consistency check
actually runs at plan time instead of failing with an `Unsupported attribute` error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant